Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Settings] Settings UX enhancements #12912

Merged
merged 6 commits into from
Aug 26, 2021

Conversation

niels9001
Copy link
Contributor

@niels9001 niels9001 commented Aug 26, 2021

Summary of the Pull Request

#12867 - Changed Severity to Normal, but added blue highlight color so it stands out better.

Before:
Before

After:
After
After2

#12865 - Added an icon that shows that there's an error without expanding the plug-in settings. We can replace this later on with a InfoBadge whenever we upgrade to WinUI 2.7.
ErrorWarningSIgnPlugins

#12874 - Added a border around the app name
image

Quality Checklist

Contributor License Agreement (CLA)

A CLA must be signed. If not, go over here and sign the CLA.

@niels9001 niels9001 marked this pull request as ready for review August 26, 2021 15:25
@niels9001
Copy link
Contributor Author

@jaimecbernardo @dedavis6797 would be great to get this one in before v0.45

@htcfreek
Copy link
Collaborator

htcfreek commented Aug 26, 2021

@niels9001

@SeraphimaZykova
Copy link
Collaborator

Should the An update is ready to install state have the same severity as An update is available?

image

Copy link
Collaborator

@jaimecbernardo jaimecbernardo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Great work!

@SeraphimaZykova
Copy link
Collaborator

For An update is available state the color is still green too. Looks like colors were added but aren't used anywhere.

@Jay-o-Way

This comment has been minimized.

@niels9001
Copy link
Contributor Author

@seraphimaz as @Jay-o-Way pointed out, I forgot push a commit changing the severity level. The ready to install banner will stay as is!

@niels9001
Copy link
Contributor Author

@niels9001

  • Can we write in KBM for <appName> to be in line with <key1> to <key2>? Result would be win+e to F1 for word.exe.

Holding out for now.. it might introduce language complexities for languages that have a different grammatical build up of the sentence.

  • Does the icon have a tool tip as suggested?

InfoBadges normally do not provide tooltips (see WinUI spec). Clicking on it will show open the expander and show the message. I don't see the value of showing it twice, plus we'd need to make it keyboard accessible as well.

@htcfreek
Copy link
Collaborator

@seraphimaz as @Jay-o-Way pointed out, I forgot push a commit changing the severity level. The ready to install banner will stay as is!

But this is an informative message because the update isn't finished at this time.

I think we should have/think like this: Ready (information) => Installing (information) => Up to date (success)

@jaimecbernardo
Copy link
Collaborator

I can confirm it's blue after the latest commit:

image

And then green:

image

@jaimecbernardo
Copy link
Collaborator

@niels9001 ,
Is current state of the PR as you intended? blue, then green?

Copy link
Collaborator

@SeraphimaZykova SeraphimaZykova left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@niels9001 niels9001 merged commit da46b90 into master Aug 26, 2021
@niels9001 niels9001 deleted the users/niels9001/settings-improvements branch August 26, 2021 17:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants